Codecov Report

Attention: Patch coverage is 92.59259% with 8 lines in your changes missing coverage. Please review.

Project coverage is 88.86%. Comparing base (2d829b4) to head (c619d27).
Report is 7 commits behind head on master.

@@           Coverage Diff           @@
##           master     #606   +/-   ##
=======================================
  Coverage   88.86%   88.86%           
=======================================
  Files          48       48           
  Lines       15474    15551   +77     
=======================================
+ Hits        13751    13820   +69     
- Misses       1723     1731    +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Uhh, changing the stable-variant-ids test is a hinting that it breaks backwards compatibility. Moving the downloaded file into a subdirectory could break existing recipes. Even if the file is extracted, the recipe can still rely on the file being present today. So at least we must have a policy to guard this.

I would propose to have the downloaded file not in a magic hidden directory but instead completely outside of workspace. We could have a downloads next to workspace that stores the downloaded file and the canary. That would make it completely transparent. No need to exclude the directory from hashing and the workspace appears clean...

I would propose to have the downloaded file not in a magic hidden directory but instead completely outside of workspace.

I also thought about this. The main reason for placing it inside was the attic logic for workspaces. I'm unsure how to thread the download-folder files if the recipe changes. Simply delete it? Or do we need some attic logic there as well?

I would propose to have the downloaded file not in a magic hidden directory but instead completely outside of workspace.

I also thought about this. The main reason for placing it inside was the attic logic for workspaces. I'm unsure how to thread the download-folder files if the recipe changes.

Good point. Indeed, this will require some extra logic to keep the downloads directory from collecting garbage. But this will probably enable further optimizations in the future. If the downloaded file is already available locally (e.g. when using a preMirror), we can directly extract such archives in the future without even copying the file in the workspace.

Simply delete it? Or do we need some attic logic there as well?

I think we can simply delete it. The "downloads" directory probably needs to have the same structure as the "workspace". This is required anyway because there might be multiple url SCMs in the same workspace. I guess adding a postAttic method to SCMs which is called in the attic code path can simply delete the downloaded file.

Thanks!